feat: introduce the Ruff pre-commit hook to replace a bunch of other code formatters and linter hooks by jenstroeger · Pull Request #1419 · oracle/macaron

jenstroeger · 2026-06-17T02:23:23Z

Summary

This change replaces black, bandit, isort, flake8, and pyupgrade git hooks with a single ruff hook.

Description of changes

Ruff is supposed to be a drop-in replacement for the aforementioned checkers, linters, and code formatters and it mostly works. However, it also

reformatted a few lines of code;
complained about a few issues here and there;
complained about FooBarException which should be named FooBarError which could be considered a breaking change;
required renaming bandit comments nosec Bxxx to noqa: Sxxx.

In addition to the rules covering the existing Macaron setup, Ruff provides a bunch of new rules and checkers (e.g. boolean trap) that are probably interesting to add… 🤓

Related issues

n/a

Checklist

I have reviewed the contribution guide.
My PR title and commits follow the Conventional Commits convention.
My commits include the "Signed-off-by" line.
I have signed my commits following the instructions provided by GitHub. Note that we run GitHub's commit verification tool to check the commit signatures. A green verified label should appear next to all of your commits on GitHub.
I have updated the relevant documentation, if applicable.
I have tested my changes and verified they work as expected.

…code formatters and linter hooks Signed-off-by: Jens Troeger <jens.troeger@light-speed.de>

jenstroeger · 2026-06-17T04:56:55Z

@behnazh-w in addition to the existing checks, perhaps it would make sense to add the following:

FBT (check for boolean trap)
DTZ (ban the usage of unsafe naive datetime class)
ICN (how certain packages should be imported or aliased)
PIE (assortment of various checks, partially overlaps with others like Bugbear or pylint)
SIM (various tips about simplifying code)
SLOT (ensure __slots__ for subclasses of immutables like str)
TID (additional tidying up of imports)
PERF (see also perflint, a plugin we removed due to inactivity)
DOC and D (check and enforce docstring comments)
F (check various lint and flakes)
PL (this is pylint but also consider Implement Pylint astral-sh/ruff#970)
FURB (similar to pyupgrade)
RUF (these are additional checks implemented by Ruff)

Also, note that there’s a CPY (copyright) rule which might replace this:

macaron/.pre-commit-config.yaml

Lines 203 to 211 in 109edd2

    
           # Checks the copyright header for .js, .py, and .java, etc. files. 
        
           - repo: local 
        
             hooks: 
        
             - id: copyright-checker 
        
               name: Copyright checker 
        
               entry: scripts/dev_scripts/copyright-checker.sh 
        
               language: system 
        
               always_run: true 
        
               pass_filenames: false

jenstroeger · 2026-06-18T01:49:44Z

+            "Invalid_status_on_skipped",
+            [],
+            [],
+            status_on_skipped,  # type: ignore


@behnazh-w I am a little confused about this comment. For one, it’s a blanket ignore and it doesn’t specify what error is being ignored here; this line should be

status_on_skipped, # type: ignore[arg-type]

But then the question arises: why is this argument type being ignored? Looking at the signature for the initializer

macaron/src/macaron/slsa_analyzer/checks/base_check.py

Lines 24 to 34 in 109edd2

class BaseCheck:

"""This abstract class is used to implement Checks in Macaron."""

def __init__(

self,

check_id: str = "",

description: str = "",

depends_on: list[tuple[str, CheckResultType]] | None = None,

eval_reqs: list[ReqName] | None = None,

result_on_skip: CheckResultType = CheckResultType.SKIPPED,

) -> None:

the type for the status_on_skipped variable should be

macaron/src/macaron/slsa_analyzer/checks/check_result.py

Lines 15 to 26 in 109edd2

class CheckResultType(str, Enum):

"""This class contains the types of a check result."""

PASSED = "PASSED"

FAILED = "FAILED"

# A check is skipped from another check's result.

SKIPPED = "SKIPPED"

# A check is disabled from the user configuration.

DISABLED = "DISABLED"

# The result of the check is unknown or Macaron cannot resolve the

# implementation of this check.

UNKNOWN = "UNKNOWN"

However, that status_on_skipped variable is the test function‘s argument

macaron/tests/slsa_analyzer/checks/test_registry.py

Lines 156 to 162 in 109edd2

@given(

lists(

one_of(none(), text(), integers(), tuples(), binary(), booleans()),

min_size=1,

)

)

def test_exit_on_invalid_status_on_skipped(self, status_on_skipped: SearchStrategy) -> None:

declared as a Hypothesis SearchStrategy. But that’s not quite right because @given feeds values from strategies into the test function and not the strategy object. Thus, the function argument receives a list[None | str | int | tuple | bytes | bool] but that still doesn’t match the signature’s CheckResultType. So shouldn’t this test look something like

@given(st.sampled_from(list(CheckResultType))) def test_exit_on_invalid_status_on_skipped(self, status_on_skipped: CheckResultType) -> None: ...

I’ve not tested this.

I then grepped through the code base and there are heaps of blanket type ignores. I think it would therefore make sense to open an issue to review all these blanket ignores, attempt to address them by fixing up the code (see above), and if that’s not possible constrain the ignore to only the error that’s being ignored. By doing so, other type errors will be surfaced if they occur instead of silently swallowing all errors.

oracle-contributor-agreement Bot added the OCA Verified All contributors have signed the Oracle Contributor Agreement. label Jun 17, 2026

feat: introduce the Ruff pre-commit hook to replace a bunch of other …

8e8615a

…code formatters and linter hooks Signed-off-by: Jens Troeger <jens.troeger@light-speed.de>

jenstroeger force-pushed the ruff branch from 4119f31 to 8e8615a Compare June 17, 2026 04:15

chore: fix tests

1b181d5

jenstroeger commented Jun 18, 2026

View reviewed changes

jenstroeger force-pushed the ruff branch from 06abe05 to 1b181d5 Compare June 18, 2026 01:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: introduce the Ruff pre-commit hook to replace a bunch of other code formatters and linter hooks#1419

feat: introduce the Ruff pre-commit hook to replace a bunch of other code formatters and linter hooks#1419
jenstroeger wants to merge 2 commits into
oracle:mainfrom
jenstroeger:ruff

jenstroeger commented Jun 17, 2026 •

edited

Loading

Uh oh!

jenstroeger commented Jun 17, 2026

Uh oh!

jenstroeger Jun 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

	class BaseCheck:
	"""This abstract class is used to implement Checks in Macaron."""

	def __init__(
	self,
	check_id: str = "",
	description: str = "",
	depends_on: list[tuple[str, CheckResultType]] \| None = None,
	eval_reqs: list[ReqName] \| None = None,
	result_on_skip: CheckResultType = CheckResultType.SKIPPED,
	) -> None:

	class CheckResultType(str, Enum):
	"""This class contains the types of a check result."""

	PASSED = "PASSED"
	FAILED = "FAILED"
	# A check is skipped from another check's result.
	SKIPPED = "SKIPPED"
	# A check is disabled from the user configuration.
	DISABLED = "DISABLED"
	# The result of the check is unknown or Macaron cannot resolve the
	# implementation of this check.
	UNKNOWN = "UNKNOWN"

	@given(
	lists(
	one_of(none(), text(), integers(), tuples(), binary(), booleans()),
	min_size=1,
	)
	)
	def test_exit_on_invalid_status_on_skipped(self, status_on_skipped: SearchStrategy) -> None:

Conversation

jenstroeger commented Jun 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Description of changes

Related issues

Checklist

Uh oh!

jenstroeger commented Jun 17, 2026

Uh oh!

jenstroeger Jun 18, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

jenstroeger commented Jun 17, 2026 •

edited

Loading